Skip to content

Worktree split geological model 2 - #298

Closed
lachlangrose wants to merge 11 commits into
masterfrom
worktree-split-geological-model-2
Closed

Worktree split geological model 2#298
lachlangrose wants to merge 11 commits into
masterfrom
worktree-split-geological-model-2

Conversation

@lachlangrose

Copy link
Copy Markdown
Member

No description provided.

- Added a new method `_validate_intrusion_inputs` in `GeologicalModel` to validate inputs for intrusions, ensuring necessary data is present before processing.
- Updated `_build_intrusion` to call the new validation method, improving error handling for missing data.
- Refactored `IntrusionBuilder.create_geometry_using_geometric_scaling` to clarify that geometric scaling is not currently implemented, raising a `NotImplementedError` immediately.
- Simplified threshold handling in `IntrusionFeature` by removing redundant checks for marginal faults.
- Removed the unused `intrusion_support_functions.py` file to clean up the codebase.
- Updated tests in `test_intrusions.py` to cover new validation logic, ensuring clear error messages for missing data and parameters.
- Added regression tests for previously silent errors related to weight handling and geometric scaling.
…ualisation workspace package

Ports the loopstructural-visualisation repo in as a uv-workspace member,
following the src-layout pattern proven by packages/loop_common and
packages/loop_interpolation in Stage 2. Import name stays
loopstructuralvisualisation; only the on-disk container path changes.

- Routed the one print() call in __init__.py's trame-ui import guard
  through LoopStructural.utils.getLogger, consistent with the rest of
  the ecosystem's logging conventions.
- Left LoopStructural.datatypes imports in _3d_viewer.py as-is rather
  than swapping to LoopStructural.geometry: that module only exists in
  this repo's unreleased dev tree, not in any published LoopStructural
  release, and the package's declared floor is LoopStructural>=1.6.17.
Ports Loop3D/map2loop (upstream commit 078f8a6, master @ 3.3.1) into this
repo as packages/map2loop, following the packages/loop_interpolation
src-layout convention (Stage 2 of ROADMAP.md), for Stage 4's map2loop half.

- Source moved from flat map2loop/ to src/map2loop/; test suite ported to
  packages/map2loop/tests/ unchanged apart from one CWD-relative fixture
  path fix (tests/sampler/geo_test.csv -> path relative to the test file)
  needed because tests now run from the monorepo root, not map2loop's own
  repo root.
- pyproject.toml written from the loop_interpolation template (build
  system, classifiers, src-layout packages.find, tests extra) but keeps
  map2loop's own real name/version/requires-python/dependencies.
- Dependency-declaration gaps fixed (imported at module level but
  undeclared upstream): pandas and packaging. GDAL (osgeo) is also a hard,
  module-level import but is deliberately left undeclared, matching
  upstream's own convention of treating it as an externally-provisioned
  (conda/system) dependency rather than a pip one -- their own
  dependencies.txt explicitly skips it in check_all_dependencies(), and
  pip-building it from source fails without system libgdal headers.
- dependencies.txt (consulted at runtime by map2loop's own import-time
  version-compatibility self-check) copied to src/dependencies.txt to
  match the relative path the existing code resolves it from, and pytest
  removed from it since map2loop's own self-check otherwise hard-requires
  pytest to be installed for a plain (non-tests-extra) install to import
  successfully.

Verified in an isolated uv venv: `import map2loop` succeeds, and
`pytest packages/map2loop/tests` gives 78 passed, 4 failed, 1 skipped, 1
collection error -- all 5 non-passing outcomes trace back to the absence
of native GDAL bindings in this pip/Windows-only verification environment
(4 of them) or a monorepo-embedding artifact where one test's internal
`pytest.main()` call with no explicit path picks up this repo's own
unrelated tests/conftest.py pytest_plugins (1 of them), not to the port
itself.

Audited for loop_common type reuse per ROADMAP.md Stage 4: no swap made.
map2loop's bounding_box is a plain dict of map-CRS extents threaded
through dozens of call sites, architecturally unrelated to
loop_common.geometry.BoundingBox's local/global mesh-frame transform
model; its own map2loop.logging module is already map2loop's public,
already-used API surface, so routing its handful of ad-hoc print()
statements through loop_common.logging would need adding loop-common as a
new hard dependency for a cosmetic swap; no custom point/orientation data
structures exist to compare against loop_common.observations (dip/dip
direction data stays as GeoDataFrame columns throughout). No clean,
low-risk win found.
Completes Stage 4 (ROADMAP.md, outcomes 5/7) for map2loop and
loopstructural-visualisation, following on from the two ported packages
already merged in from agent worktrees (packages/map2loop,
packages/loopstructural_visualisation).

- Add both to [tool.uv.sources]; loopstructural_visualisation also needs
  LoopStructural = { workspace = true } since it depends on LoopStructural
  itself, which uv treats as an implicit workspace member once referenced
  by name.
- CI: loopstructural_visualisation joins the existing packages-test matrix;
  map2loop gets its own ubuntu-only job (map2loop-test) with an
  apt-installed libgdal, since GDAL has no pip wheel on Windows/macOS and
  upstream map2loop itself only tests via conda for the same reason.
- Add a minimal import-smoke test for loopstructural_visualisation, which
  shipped with zero test coverage upstream.
- Wire both into pypi.yml sdist/publish, release-please-config.json +
  .release-please-manifest.json (loopstructural_visualisation uses
  extra-files to bump version.py's __version__, since it has no static
  pyproject version field), and grandfather D/ANN ruff rules for both
  (matching the loop_common/loop_interpolation precedent).
- Correct ROADMAP.md: loopresources (Stage 4a) was never actually merged
  to master, despite a prior status-log entry claiming otherwise.

Verified: uv sync resolves cleanly for both packages standalone and as a
full workspace sync; tests/unit 695 passed/4 skipped (zero regressions).
…raphic_column from wiping state

evaluate_gradient's tetrahedron-refinement loop set resolved=True
unconditionally after one pass, so it never actually retried after
shrinking the tetrahedron near a fault. set_stratigraphic_column
cleared the model's stratigraphic column before unconditionally
raising DeprecationWarning, destroying existing state on every call.
GeologicalModel mixed recipe/pickle I/O in with feature-construction
orchestration, evaluation, and export logic (2580 lines total). Move
the recipe/JSON and dill/pickle serialization logic into a standalone
ModelSerializer, keeping every GeologicalModel public method as a
thin unchanged-signature wrapper so the CI-enforced stable API
surface (API.md, tests/unit/test_public_api_contract.py) is
unaffected.
Move surface/block-model export logic (get_fault_surfaces,
get_stratigraphic_surfaces, get_block_model, save) into a standalone
ModelExporter, continuing the GeologicalModel decomposition. Public
methods remain unchanged thin wrappers.
Move fault/unconformity relationship bookkeeping (_add_faults,
_add_domain_fault_above/below, _add_unconformity_above,
add_unconformity, add_onlap_unconformity) into a standalone
FeatureRelationshipManager, continuing the GeologicalModel
decomposition. Public methods remain unchanged thin wrappers;
_add_feature stays on GeologicalModel since it owns
features/feature_name_index mutation directly.
Move the seven _build_* feature-construction methods (foliation,
fold frame, folded foliation, folded fold frame, intrusion, domain
fault, fault) -- collectively ~1000 lines, the largest remaining
chunk of GeologicalModel -- into a standalone ModelFeatureFactory.
The FeatureBuilderRegistry wiring at the bottom of geological_model.py
now dispatches to ModelFeatureFactory.build_* instead of the removed
private methods. Public create_and_add_* wrappers are unchanged;
their docstring cross-references are updated to point at the new
factory location. geological_model.py drops from 2579 to 1505 lines.
@lachlangrose

Copy link
Copy Markdown
Member Author

Split into a stacked series of smaller PRs for easier review:

  1. fix: Updating intrusions code #300 fix: Updating intrusions code
  2. feat: add loopstructural-visualisation as packages/loopstructural_visualisation workspace package #301 feat: add loopstructural-visualisation package
  3. feat: add map2loop as packages/map2loop workspace package #302 feat: add map2loop package
  4. feat: wire map2loop and loopstructural_visualisation into the workspace #303 feat: wire map2loop/visualisation into workspace
  5. fix: correct fault-proximity gradient retry loop and stop set_stratigraphic_column from wiping state #304 fix: fault-proximity gradient + stratigraphic column
  6. refactor: extract ModelSerializer from GeologicalModel #305 refactor: extract ModelSerializer
  7. refactor: extract ModelExporter from GeologicalModel #306 refactor: extract ModelExporter
  8. refactor: extract FeatureRelationshipManager from GeologicalModel #307 refactor: extract FeatureRelationshipManager
  9. refactor: extract ModelFeatureFactory from GeologicalModel #308 refactor: extract ModelFeatureFactory

Verified the split is lossless — the merged tips of the stack reproduce the exact same tree as this branch. Closing in favour of the stack above.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant